Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cibuildwheel: move configuration to pyproject.toml #412

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

cibuildwheel: move configuration to pyproject.toml #412

wants to merge 5 commits into from

Conversation

fruch
Copy link

@fruch fruch commented Jan 23, 2025 •
edited
Loading

move the configuration to pyproject.toml, that it would
be easier to use from multiple workflow and for local usage

for example:

uv pip install cibuildwheel
CIBW_FREE_THREADED_SUPPORT=True  cibuildwheel --only cp313-manylinux_x86_64

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have provided docstrings for the public items that I want to introduce.
  • I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

@fruch fruch requested review from Lorak-mmk and dkropachev January 23, 2025 08:25
@fruch fruch mentioned this pull request Jan 23, 2025
Merged
8 tasks
@fruch fruch force-pushed the fail_if_not_ext_built branch 6 times, most recently from 1967715 to 0b856da Compare January 23, 2025 22:36
@fruch
Copy link
Author

fruch commented Jan 24, 2025

@dkropachev

See my commits to switch using pyproject.toml seems they are helping with the building situation quite a lot

It's still a work in progress (macos is not targeting correct in some)

Other issues surfaced cause failing extensions:

  • windows doesn't have uint_128 as we declare and use in Linux/macos

  • aarch64 - gcc crashing in one extension complication

We'll need to take a call on those
I don't mind skipping windows (the must compile flag), but aarch64 we must fix

@dkropachev
Copy link
Collaborator

@dkropachev

See my commits to switch using pyproject.toml seems they are helping with the building situation quite a lot

It's still a work in progress (macos is not targeting correct in some)

Other issues surfaced cause failing extensions:

  • windows doesn't have uint_128 as we declare and use in Linux/macos
  • aarch64 - gcc crashing in one extension complication

We'll need to take a call on those I don't mind skipping windows (the must compile flag), but aarch64 we must fix

Thanks @fruch , it fixed ubuntu, working on aarch, it looks like all major building binaries are affected: patchelf, gcc, c++.
Bumping to ubuntu-24.04 did not help, trying to update apt packages.

@fruch
Copy link
Author

fruch commented Jan 25, 2025

@dkropachev

See my commits to switch using pyproject.toml seems they are helping with the building situation quite a lot

It's still a work in progress (macos is not targeting correct in some)

Other issues surfaced cause failing extensions:

  • windows doesn't have uint_128 as we declare and use in Linux/macos
  • aarch64 - gcc crashing in one extension complication

We'll need to take a call on those I don't mind skipping windows (the must compile flag), but aarch64 we must fix

Thanks @fruch , it fixed ubuntu, working on aarch, it looks like all major building binaries are affected: patchelf, gcc, c++.
Bumping to ubuntu-24.04 did not help, trying to update apt packages.

You are missing the point, compilation is done inside docker, we can try updating packing inside the before build command, or find out what the root cause, it's only one extension that crashes

With the new setup cibuildwheel and docker, you can run the aarch64 locally (it would be slow be easier to debug)

@dkropachev
Copy link
Collaborator

@dkropachev
See my commits to switch using pyproject.toml seems they are helping with the building situation quite a lot
It's still a work in progress (macos is not targeting correct in some)
Other issues surfaced cause failing extensions:

  • windows doesn't have uint_128 as we declare and use in Linux/macos
  • aarch64 - gcc crashing in one extension complication

We'll need to take a call on those I don't mind skipping windows (the must compile flag), but aarch64 we must fix

Thanks @fruch , it fixed ubuntu, working on aarch, it looks like all major building binaries are affected: patchelf, gcc, c++.
Bumping to ubuntu-24.04 did not help, trying to update apt packages.

You are missing the point, compilation is done inside docker, we can try updating packing inside the before build command, or find out what the root cause, it's only one extension that crashes

With the new setup cibuildwheel and docker, you can run the aarch64 locally (it would be slow be easier to debug)

It looks like I found cure for aarch https://github.com/scylladb/python-driver/actions/runs/12970378006/job/36175553064?pr=411, ran pass cp310-manylinux_aarch64 wheel

@dkropachev
Copy link
Collaborator

fixes macos as well, making a glimpse at windows.

@fruch fruch force-pushed the fail_if_not_ext_built branch 2 times, most recently from 47d4409 to b2c6171 Compare January 26, 2025 13:31
@fruch fruch force-pushed the fail_if_not_ext_built branch from b2c6171 to f36dbf2 Compare January 30, 2025 13:16
fruch added 3 commits January 30, 2025 16:02
@fruch
cibuildwheel: move configuration to pyproject.toml
bb76d8a 
move the configuration to pyproject.toml, that it would
be easier to use from multiple workflow and for local usage

for example:
```
uv pip install cibuildwheel
CIBW_FREE_THREADED_SUPPORT=True  cibuildwheel --only cp313-manylinux_x86_64
```
@fruch
windows: fix building of c_shard_info.pyx
80b044b 
without this, that `c_shard_info.c` fails to compile like the following:

```
  cassandra\c_shard_info.c(3083): error C2065: '__uint128_t': undeclared identifier
  cassandra\c_shard_info.c(3083): error C2146: syntax error: missing ')' before identifier '__pyx_v_biased_token'
  cassandra\c_shard_info.c(3083): error C2059: syntax error: ')'
  cassandra\c_shard_info.c(3083): error C2059: syntax error: ')'
  cassandra\c_shard_info.c(3083): error C2059: syntax error: ')'
```
@fruch
add setuptools-scm into pyproject.toml
e5ae035 
with this change, we version string of the driver
would alway follow the git tag
it would add extra information if it's development
branch without a tag like git sha and date of the build

Ref: https://setuptools-scm.readthedocs.io/
@fruch fruch force-pushed the fail_if_not_ext_built branch from aacb150 to e5ae035 Compare January 30, 2025 14:02
@fruch fruch changed the title git actions: fail build if extensions aren't built cibuildwheel: move configuration to pyproject.toml Jan 30, 2025
@fruch
Copy link
Author

fruch commented Jan 30, 2025

@dkropachev

in the PR now left a just:

  • the refactor of cibuildwheel configuration
  • a better fix for the windows issue (that won't need switching compilers)
  • update to use setuptools_scm for versioning (docs part still need to happen manually)

@fruch
Copy link
Author

fruch commented Jan 30, 2025

@dkropachev

also the pre release workflow, is setup for python 3.14, which isn't available, but it would not fail until there would be a beta version available (i.e. we'll need to update cibuildwheel for that to happen)

@fruch fruch marked this pull request as ready for review January 30, 2025 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lorak-mmk Lorak-mmk Awaiting requested review from Lorak-mmk

@dkropachev dkropachev Awaiting requested review from dkropachev

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fruch @dkropachev